chore: rewrite project structure#108
Conversation
hrodmn
left a comment
hrodmn
left a comment
There was a problem hiding this comment.
@jjfrench thanks for taking this on! I have a few ideas for you to consider but don't have any very strong opinions here.
I think the most actionable one might be to consider breaking the big global Config class into smaller specific configs for various components. Right now we take the big Config and construct things like PgstacDbConfig directly from the Config properties. I think it would be cleaner and easier to maintain if we did not need to pass properties through the global Config to the lower level configs. That way if we change things or add a property we would only need to add it in one place rather than adding it to Config and adding it to XXXConfig.
We don't need to do it in this PR but it is probably time to break PgstacInfra into some smaller constructs! That thing is a beast.
Co-authored-by: Henry Rodman <henry.rodman@gmail.com>
| def build_stack_name(self, service_id: str) -> str: | ||
| return f"MAAP-STAC-{self.stage}-{service_id}" |
There was a problem hiding this comment.
Let's double check this is consistent so the stacks don't get re-created from scratch (maybe you already tested that)
There was a problem hiding this comment.
I've been running cdk diff's to check - only the pgstac/user pgstac stacks lambda version modifications and a slight permission consolidation
| user_stac_catalog_transactions_enabled: Optional[bool] = None | ||
| user_stac_catalog_transactions_auth_mode: Optional[str] = None | ||
| user_stac_catalog_transactions_auth_secret_arn: Optional[str] = None |
There was a problem hiding this comment.
We could just drop the _enabled type of settings and rely on the required fields being set, throw an error if any are missing.
| user_stac_catalog_transactions_enabled: Optional[bool] = None | |
| user_stac_catalog_transactions_auth_mode: Optional[str] = None | |
| user_stac_catalog_transactions_auth_secret_arn: Optional[str] = None | |
| user_stac_catalog_transactions_auth_mode: Optional[str] = None | |
| user_stac_catalog_transactions_auth_secret_arn: Optional[str] = None |
There was a problem hiding this comment.
Just need to remember to clean up the Github environment variables afterwards
| user_stac_collection_transactions_enabled: Optional[bool] = None | ||
| user_stac_collection_transactions_auth_mode: Optional[str] = None | ||
| user_stac_collection_transactions_auth_secret_arn: Optional[str] = None | ||
| user_stac_collection_transactions: Optional[CollectionTransactionsConfig] = None |
There was a problem hiding this comment.
Does this need to be an attribute or could it just be defined as a property? Seems to be derived from other attributes that are defined with env vars.
2 participants
Description
Rewrites the project structure in python rather than TypeScript and Node - aims to not change any of the infrastructure
(would close open dependabot security issues for javascript)